BB2-4675: Add SAMHSA checkbox to v3 permissions screen by JamesDemeryNava · Pull Request #1607 · CMSgov/bluebutton-web-server

JamesDemeryNava · 2026-05-22T19:12:22Z

JIRA Ticket:
BB2-4675

What Does This PR Do?

Adds a checkbox on the v3 permissions screen, if the app has any ExplanationOfBenefit scopes. The checkbox is not selected by default. If the user leaves the checkbox unchecked, then the include_samhsa value on the oauth2_provider_accesstoken_extension record will be false, SAMHSA data will be filtered out of v3 EOB responses, and v1/2 EOB calls will be blocked for that token. If the user checks the checkbox, then the include_samhsa value on the oauth2_provider_accesstoken_extension record will be true, SAMHSA data will NOT be filtered out of v3 EOB responses, and v1/2 EOB calls will be allowed.

What Should Reviewers Watch For?

Is test coverage sufficient?
Any concerns about the caching strategy?

If you're reviewing this PR, please check for these things in particular:

Validation

Pull down the branch and build the project
Run make migrate
In PgAdmin or DBeaver, confirm the dot_ext_auth_flow_tracking table now exists.
- In Postman, make sure your app has EOB scopes, go through a v3 auth flow
- Leave the SAMHSA checkbox unchecked
- Query the database for the most recent oauth2_provider_accesstoken_extension record, confirm include_samhsa is false
- Make a v3 EOB search call, confirm that _security:not=42CFRPart2 is included in the link attribute of the response
- Refresh the token, confirm the oauth2_provider_accesstoken_extension (it will be a new record, as the access token will have been deleted), still has include_samhsa of false
- Go through another v3 auth flow, this time, check the SAMHSA checkbox.
- Query the database for the most recent oauth2_provider_accesstoken_extension record, confirm include_samhsa is true
- Make a v3 EOB search call, confirm that _security:not=42CFRPart2 is NOT included in the link attribute of the response
- Refresh the token, confirm the oauth2_provider_accesstoken_extension (it will be a new record, as the access token will have been deleted), still has include_samhsa of true
- Query dot_ext_auth_flow_tracking again, make sure no records are returned
- Go through a v2 auth flow, make sure the oauth2_provider_accesstoken_extension has include_samhsa equal to True
- Make sure integration tests pass (with bfd=sbx)

What Security Implications Does This PR Have?

Please indicate if this PR does any of the following:

Adds any new software dependencies
Modifies any security controls
Adds new transmission or storage of data
Any other changes that could possibly affect security?

Yes, one or more of the above security implications apply. This PR must not be merged without the ISSO or team
security engineer's approval.

Any Migrations?

Yes, there are migrations
- The migrations should be run PRIOR to the code being deployed
- The migrations should be run AFTER the code is deployed
- There is a more complicated migration plan (downtime,
  etc)
[] No migrations

…-samhsa-checkbox

ryan-morosa

Nice work! I left a few comments and I think the testing coverage is sufficient and the functionality works as described. I wasn't able to do a case through postman where there was a code for the cache - is there a way this can get tested?

JamesDemeryNava · 2026-05-28T18:21:50Z

Nice work! I left a few comments and I think the testing coverage is sufficient and the functionality works as described. I wasn't able to do a case through postman where there was a code for the cache - is there a way this can get tested?

Thanks! For me, I always get a value for code in both form_valid of AuthorizationView and the post of TokenView if I am going through an auth flow. If you do a refresh token flow, code will be a None value in the post of TokenView, and we won't hit form_valid of AuthorizationView. For refresh tokens, we just grab the prior include_samhsa value, and apply that to the new access_token_extension record.

To me, if you did not check the checkbox, and include_samhsa was false on the resulting access_token_extension record, that means the caching is working. Though I am hoping to get @jimmyfagan's opinion on the caching strategy before merging this.

ryan-morosa · 2026-05-28T18:31:52Z

Nice work! I left a few comments and I think the testing coverage is sufficient and the functionality works as described. I wasn't able to do a case through postman where there was a code for the cache - is there a way this can get tested?

Thanks! For me, I always get a value for code in both form_valid of AuthorizationView and the post of TokenView if I am going through an auth flow. If you do a refresh token flow, code will be a None value in the post of TokenView, and we won't hit form_valid of AuthorizationView. For refresh tokens, we just grab the prior include_samhsa value, and apply that to the new access_token_extension record.

To me, if you did not check the checkbox, and include_samhsa was false on the resulting access_token_extension record, that means the caching is working. Though I am hoping to get @jimmyfagan's opinion on the caching strategy before merging this.

Totally. I think it makes sense to have an event-based caching strategy like you have here where we update the database and then remove the previous one from the cache. But I'd be open to other strategies too.

…-samhsa-checkbox

ryan-morosa

Left a few comments - overall nice job and all tests and functionality look good!

…-samhsa-checkbox

Copilot

Pull request overview

This PR adds an opt-in SAMHSA sharing choice to the v3 permissions screen for apps requesting any ExplanationOfBenefit scopes, persists that choice through the OAuth authorization→token exchange, and uses it to control SAMHSA filtering behavior in v3 EOB responses and v1/v2 EOB access.

Changes:

Adds a SAMHSA checkbox to the v3 authorize page (only shown when EOB scopes are present).
Introduces a dot_ext_auth_flow_tracking table to temporarily persist the checkbox value across the auth-code redirect, and wires token issuance to create/update AccessTokenExtension.include_samhsa accordingly.
Updates/extends unit & integration tests and removes the old AccessTokenExtension creation signal.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
templates/design_system/authorize_v3.html	Adds the SAMHSA checkbox UI on the v3 permission screen when EOB scopes are requested.
apps/fhir/bluebutton/tests/test_fhir_resources_read_search_w_validation.py	Updates integration tests to explicitly create `AccessTokenExtension` now that the auto-create signal is removed.
apps/dot_ext/views/authorization.py	Persists SAMHSA choice into `AuthFlowTracking` during authorization; reads prior value during refresh-token flow; creates extension during token issuance.
apps/dot_ext/utils.py	Adds helper to read/delete `AuthFlowTracking` and create/update `AccessTokenExtension`.
apps/dot_ext/tests/test_utils.py	Adds unit tests for the new helper.
apps/dot_ext/tests/test_models.py	Removes/adjusts model tests that previously relied on signal-based extension creation.
apps/dot_ext/tests/test_authorization_token.py	Adds tests for retrieving prior `include_samhsa` during refresh-token flow.
apps/dot_ext/signals.py	Removes AccessToken post-save receiver that created `AccessTokenExtension` unconditionally.
apps/dot_ext/models.py	Adds `AuthFlowTracking` model.
apps/dot_ext/migrations/0014_authflowtracking.py	Adds migration to create `dot_ext_auth_flow_tracking`.
apps/dot_ext/forms.py	Adds `share_samhsa_data` to the allow/consent form so the posted checkbox value is captured.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jadudm · 2026-06-02T16:59:22Z

+    def _retrieve_prior_include_samhsa_value(self, grant_type: str, request: HttpRequest) -> bool:
+        prior_include_samhsa = True
+        if grant_type == 'refresh_token':
+            refresh_token_str = request.POST.get('refresh_token')
+            refresh_token = get_refresh_token_model().objects.get(token=refresh_token_str)
+            try:
+                prior_access_token_extension = AccessTokenExtension.objects.get(
+                    access_token_id=refresh_token.access_token_id
+                )
+                prior_include_samhsa = prior_access_token_extension.include_samhsa
+            except AccessTokenExtension.DoesNotExist:
+                # this case indicates it was an access token created before the access token extension was added
+                log.info(f'No access token extension for access token id: {refresh_token.access_token_id}')
+        return prior_include_samhsa


…-samhsa-checkbox

ryan-morosa

I am good with these changes. I'm not sure if we can merge this yet though until we fix rotating the credentials from the recent incident and maybe get to test it one more time. That's why I'm holding off on approval for now.

…fusing

JamesDemeryNava · 2026-06-03T15:08:04Z

    code_challenge = forms.CharField(required=False, widget=forms.HiddenInput())
    code_challenge_method = forms.CharField(required=False, widget=forms.HiddenInput())
    share_demographic_scopes = forms.CharField(required=False)
+    share_samhsa_data = forms.BooleanField(required=False)


This change allowed us to not have the bad conditional check in authorization.py

…-samhsa-checkbox

ryan-morosa · 2026-06-04T14:20:59Z


+        # Default the user sharing their SAMHSA data to True, and if it is v3, check to see what the value is
+        user_approves_sharing_samhsa_data = True
+        if self.version == Versions.V3:


This is a lot cleaner and better understood - nice refactor

ryan-morosa

Refactor looks good - changes approved

…" (#1619) This reverts commit e1b0b7b.

* BB2-4675: Add SAMHSA checkbox to v3 permissions screen * Use cache.add instead of cache.set, modify default for code * Address PR feedback - add docstrings * Add a new table to track user SAMHSA preferences rather than use caching * Address PR feedback * Remove commented out line * Address copilot feedback * Convert share_samhsa_data to a BooleanField to make the code less confusing

…" (#1619) This reverts commit e1b0b7b.

BB2-4675: Add SAMHSA checkbox to v3 permissions screen

047c5e6

JamesDemeryNava commented May 22, 2026

View reviewed changes

Comment thread apps/dot_ext/signals.py

Use cache.add instead of cache.set, modify default for code

e3ce6ed

JamesDemeryNava commented May 22, 2026

View reviewed changes

Comment thread apps/dot_ext/views/authorization.py Outdated

JamesDemeryNava added 2 commits May 27, 2026 08:21

Merge branch 'master' into jamesdemery/bb2-4675-v3-permissions-screen…

20ff8f8

…-samhsa-checkbox

Merge branch 'master' into jamesdemery/bb2-4675-v3-permissions-screen…

e7f2c17

…-samhsa-checkbox

ryan-morosa self-assigned this May 28, 2026

JamesDemeryNava added 2 commits May 28, 2026 08:23

Merge branch 'master' into jamesdemery/bb2-4675-v3-permissions-screen…

0376b94

…-samhsa-checkbox

Merge branch 'master' into jamesdemery/bb2-4675-v3-permissions-screen…

0bb17cd

…-samhsa-checkbox